Skip to content

Use direct hex decoding for hash strings#33

Merged
achamayou merged 8 commits into
mainfrom
copilot/check-sscanf-return-value
Jun 5, 2026
Merged

Use direct hex decoding for hash strings#33
achamayou merged 8 commits into
mainfrom
copilot/check-sscanf-return-value

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 5, 2026

Hash string parsing relied on sscanf plus strtoul, requiring an intermediate byte buffer and implicit partial-parse handling. The parser now validates and decodes each hex nibble directly.

  • Parsing
    • Replaced sscanf/strtoul with explicit hex digit decoding.
    • Preserves rejection of invalid characters and odd partial-byte matches.
    • Removes unused C parsing includes.
if (!decode_hex_digit(s[2 * i], high) ||
    !decode_hex_digit(s[2 * i + 1], low))
{
  throw std::runtime_error("invalid hash string");
}
bytes[i] = static_cast<uint8_t>((high << 4) | low);
  • Coverage
    • Added mixed-case hex coverage to exercise uppercase and lowercase decode paths.

@achamayou achamayou marked this pull request as ready for review June 5, 2026 13:53
Copilot AI review requested due to automatic review settings June 5, 2026 13:53
@achamayou
Copy link
Copy Markdown
Member

Importantly, the code did not check the return value of sscanf, which is what prompted this PR. But it's possible to simplify quite a bit instead.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves merkle::HashT hex-string parsing by replacing sscanf-based byte decoding with explicit per-nibble validation/decoding, making invalid and partially-invalid hex strings reliably rejected while supporting mixed-case hex input.

Changes:

  • Added a small hex-digit decoder helper and used it to decode hash strings nibble-by-nibble (rejecting any non-hex character or partial byte).
  • Extended existing unit tests to cover mixed-case hex parsing.
  • Added a dedicated coverage test to exercise valid, mixed-case, and invalid/partially-invalid hash-string parsing paths.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
merklecpp.h Replaces sscanf hex parsing with explicit hex digit decoding and validation in HashT string constructor.
test/unit_tests.cpp Adds a mixed-case hash-string constructor test case.
test/coverage.cpp Adds a focused hash-string parsing coverage test and wires it into the coverage test runner.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@achamayou achamayou merged commit 3cbfd3c into main Jun 5, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants